Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request converts CSAF provider discovery and document import from Alpine's event-based task system to Dex workflows. This enables addressability by workflow instance ID and prevents multiple concurrent instances for the same aggregator or provider.
Changes:
- Replaced CSAF event classes and tasks with Dex workflows and activities
- Updated DexEngine API to accept wildcard-bounded collection types for workflow run requests
- Modified initialization order and registration to support CSAF workflows
- Removed lock-based configuration properties in favor of workflow instance IDs
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| proto/src/main/proto/org/dependencytrack/internal/workflow/v1/argument_csaf.proto | Defines protobuf messages for CSAF workflow arguments (DiscoverCsafProvidersArg and ImportCsafDocumentsArg) |
| dex/engine-api/src/main/java/org/dependencytrack/dex/engine/api/DexEngine.java | Updated createRuns signature to accept wildcard-bounded collections |
| dex/engine/src/main/java/org/dependencytrack/dex/engine/DexEngineImpl.java | Implements updated createRuns signature |
| apiserver/src/main/java/org/dependencytrack/csaf/DiscoverCsafProvidersWorkflow.java | New workflow for CSAF provider discovery |
| apiserver/src/main/java/org/dependencytrack/csaf/DiscoverCsafProvidersActivity.java | Converted from CsafProviderDiscoveryTask to Dex activity |
| apiserver/src/main/java/org/dependencytrack/csaf/ImportCsafDocumentsWorkflow.java | New workflow for CSAF document import |
| apiserver/src/main/java/org/dependencytrack/csaf/ImportCsafDocumentsActivity.java | Converted from CsafDocumentImportTask to Dex activity with heartbeat support |
| apiserver/src/main/java/org/dependencytrack/resources/v2/CsafResource.java | Updated to trigger workflows instead of dispatching events, with conflict detection |
| apiserver/src/main/java/org/dependencytrack/dex/DexEngineInitializer.java | Registers CSAF workflows and activities |
| apiserver/src/main/java/org/dependencytrack/tasks/TaskSchedulerInitializer.java | Schedules CSAF import workflow runs for enabled providers |
| apiserver/src/main/java/org/dependencytrack/event/EventSubsystemInitializer.java | Removes CSAF event subscriptions |
| apiserver/src/main/java/org/dependencytrack/util/TaskUtil.java | Adds helper method to get cron schedule from config |
| apiserver/src/main/webapp/WEB-INF/web.xml | Reorders listeners to initialize DexEngine before TaskScheduler |
| apiserver/src/main/resources/application.properties | Removes lock-based configuration properties |
| apiserver/src/main/java/org/dependencytrack/csaf/CsafProviderDiscoveryEvent.java | Deleted - replaced by workflow argument |
| api/src/main/openapi/paths/csaf/aggregators__id__provider-discovery.yaml | Adds 409 Conflict response for concurrent workflow prevention |
| apiserver/src/test/java/org/dependencytrack/resources/v2/CsafResourceTest.java | Updated tests for workflow-based implementation with conflict scenarios |
| apiserver/src/test/java/org/dependencytrack/csaf/ImportCsafDocumentsWorkflowTest.java | New workflow test replacing task test |
| apiserver/src/test/java/org/dependencytrack/csaf/DiscoveryCsafProvidersWorkflowTest.java | New workflow test replacing task test |
| apiserver/src/test/java/org/dependencytrack/tasks/TaskSchedulerInitializerTest.java | Updated for DexEngine dependency |
Comments suppressed due to low confidence (5)
apiserver/src/main/java/org/dependencytrack/csaf/ImportCsafDocumentsActivity.java:114
- Consider adding a heartbeat call after processDocuments at line 120 as well. Currently, heartbeat is only called before batch processing (line 111 for each document, line 126 for the final batch), but not after. If processDocuments takes a long time, the activity might time out without sending heartbeats during the processing phase itself.
apiserver/src/test/java/org/dependencytrack/csaf/DiscoveryCsafProvidersWorkflowTest.java:52 - The class name "DiscoveryCsafProvidersWorkflowTest" has an inconsistent naming pattern. It should be "DiscoverCsafProvidersWorkflowTest" to match the workflow name "DiscoverCsafProvidersWorkflow" and follow the pattern of "ImportCsafDocumentsWorkflowTest" in the same package. The word "Discovery" should not have a "y" suffix here.
apiserver/src/test/java/org/dependencytrack/csaf/ImportCsafDocumentsWorkflowTest.java:78 - The beforeEach method does not call super.before(), but the test uses qm at line 145 which requires qm to be initialized by the parent's before() method. This will cause NullPointerException when the test accesses qm. Call super.before() at the beginning of beforeEach to properly initialize the test infrastructure.
apiserver/src/test/java/org/dependencytrack/csaf/ImportCsafDocumentsWorkflowTest.java:113 - The afterEach method does not call super.after(), which means event services won't be drained and qm won't be properly closed. This could cause tests to interfere with each other. Call super.after() after closing the jdbiHandle to ensure proper cleanup.
apiserver/src/test/java/org/dependencytrack/csaf/DiscoveryCsafProvidersWorkflowTest.java:103 - The beforeEach and afterEach methods do not call super.before() and super.after() respectively. While this test doesn't use qm, the parent class's after() method drains event services to prevent tests from interfering with each other. Call super.before() and super.after() to ensure proper test infrastructure initialization and cleanup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Enables provider discovery and document import to be addressable by workflow instance ID, while also effectively prevent multiple instances for the same aggregator / provider to run concurrently. https://github.com/DependencyTrack/hyades-apiserver/blob/main/dex/docs/PATTERNS.md#singletons Signed-off-by: nscuro <nscuro@protonmail.com>
nscuro
force-pushed
the
csaf-workflows
branch
from
ba579e8 to
0b11324
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Converts CSAF tasks to workflows.
Enables provider discovery and document import to be addressable by workflow instance ID, while also effectively prevent multiple instances for the same aggregator / provider to run concurrently.
https://github.com/DependencyTrack/hyades-apiserver/blob/main/dex/docs/PATTERNS.md#singletons
Addressed Issue
N/A
Additional Details
N/A
Checklist
This PR fixes a defect, and I have provided tests to verify that the fix is effectiveThis PR introduces changes to the database model, and I have updated the migration changelog accordinglyThis PR introduces new or alters existing behavior, and I have updated the documentation accordingly